Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #54 to add DevServices for Artemis JMS #55

Merged
merged 5 commits into from
Jun 29, 2022

Conversation

zhfeng
Copy link
Contributor

@zhfeng zhfeng commented Jun 26, 2022

No description provided.

@zhfeng
Copy link
Contributor Author

zhfeng commented Jun 26, 2022

Hi @middagj ,

Can you review these changes? and I'd like to include them in a new release if it makes sense.

Thanks!

@middagj
Copy link
Contributor

middagj commented Jun 26, 2022

Sure, it can take a few days

Copy link
Contributor

@middagj middagj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the addition, some comments.

@middagj
Copy link
Contributor

middagj commented Jun 28, 2022

We should be able to also use testcontainers on core integration test. If you have time, could you add that, otherwise I will add it myself. Also we don't need the broker.xml files.

@zhfeng
Copy link
Contributor Author

zhfeng commented Jun 29, 2022

yeah, I will add one in core integration test. And broker.xml is needed if we still use ArtemisTestResource.

@middagj
Copy link
Contributor

middagj commented Jun 29, 2022

yeah, I will add one in core integration test. And broker.xml is needed if we still use ArtemisTestResource.

correct but only one

I think I misunderstood the test then, I thought it was only using the testcontainers.

@middagj
Copy link
Contributor

middagj commented Jun 29, 2022

Thanks for the PR

@middagj middagj merged commit 44b6fb8 into quarkiverse:main Jun 29, 2022
@middagj
Copy link
Contributor

middagj commented Jun 29, 2022

Could you give it a test, and confirm it works as expected in your project. I will then release.

@zhfeng
Copy link
Contributor Author

zhfeng commented Jun 29, 2022

Thanks @middagj and yeah I confirm it works in my camel-quarkus -example project. Please go heading for the release.

@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 1, 2022

@middagj I just realized that ArtemisTestResource is useful on windows where docker is not supported currently. So it should be better to remove @Deprecated . I'm sorry for the inconveniences.

@middagj
Copy link
Contributor

middagj commented Jul 1, 2022

I didn't realize you added it, will remove it.

@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 1, 2022

I raise #60

@turing85 turing85 added the enhancement New feature or request label Nov 9, 2022
@turing85 turing85 linked an issue Nov 9, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add DevService support
3 participants